Skip to content

refactor: add Chunk type parameter to Stream class for type-safe streaming#42

Merged
Kamilbenkirane merged 1 commit intomainfrom
refactor/add-chunk-type-parameter-to-stream
Nov 17, 2025
Merged

refactor: add Chunk type parameter to Stream class for type-safe streaming#42
Kamilbenkirane merged 1 commit intomainfrom
refactor/add-chunk-type-parameter-to-stream

Conversation

@Kamilbenkirane
Copy link
Copy Markdown
Member

Summary

This PR refactors the Stream class to be generic over a Chunk type parameter, enabling capability-specific chunk types and improving type safety across all streaming implementations.

Changes

  • Add Chunk type parameter to Stream[Out, Params, Chunk] base class
  • Update all provider streaming implementations to use specific chunk types
    • Text generation: TextGenerationChunk
    • Image generation: ImageGenerationChunk
  • Update Client.stream() and _stream_class() return types to include Chunk parameter
  • Remove generic Chunk imports from provider implementations
  • Update all tests to use new Stream signature
  • Remove streaming modules from mypy ignore lists (now properly typed)
  • Use ApplicationMimeType.JSON constant in HTTP tests

Benefits

  • ✅ Type-safe chunk parsing per capability
  • ✅ Better IDE autocomplete and type checking
  • ✅ Cleaner provider code without generic Chunk imports
  • ✅ Improved mypy compliance

Testing

All pre-commit hooks passed:

  • ✅ Linting (Ruff)
  • ✅ Formatting (Ruff)
  • ✅ Type checking (mypy)
  • ✅ Security checks (Bandit)
  • ✅ Tests with coverage

…aming

Make Stream generic over Chunk type to enable capability-specific chunk
types, improving type safety and IDE support across all streaming
implementations.

Changes:
- Add Chunk type parameter to Stream[Out, Params, Chunk] base class
- Update all provider streaming implementations to use specific chunk types
  (TextGenerationChunk, ImageGenerationChunk) instead of generic Chunk
- Update Client.stream() and _stream_class() return types to include Chunk
- Remove generic Chunk imports from provider implementations
- Update all tests to use new Stream signature
- Remove streaming modules from mypy ignore lists (now properly typed)
- Use ApplicationMimeType.JSON constant in HTTP tests

Benefits:
- Type-safe chunk parsing per capability
- Better IDE autocomplete and type checking
- Cleaner provider code without generic Chunk imports
- Improved mypy compliance
@claude
Copy link
Copy Markdown

claude Bot commented Nov 17, 2025

Code Review: Chunk Type Parameter Refactoring

Thank you for this PR! This is a well-executed refactoring that significantly improves type safety across the streaming implementations. Here's my detailed review:

Strengths

  1. Excellent Type Safety Improvement

    • Adding the Chunk type parameter to Stream[Out, Params, Chunk] provides capability-specific chunk types
    • This enables proper type checking and IDE autocomplete for chunk handling
    • The change from generic Chunk to TextGenerationChunk and ImageGenerationChunk is semantically correct
  2. Consistent Implementation

    • All provider implementations consistently updated across:
      • Text generation providers (Anthropic, Cohere, Google, Mistral, OpenAI)
      • Image generation providers (Bytedance, OpenAI)
    • Method signatures properly updated from Chunk | None to specific chunk types
  3. Improved Code Cleanliness

    • Removing from celeste.io import Chunk from provider files eliminates confusion
    • Providers now only import their capability-specific chunk types
    • Better separation of concerns between base and capability-specific types
  4. Mypy Compliance

    • Removing streaming modules from mypy ignore lists (pyproject.toml:168, 176) demonstrates improved type safety
    • This is a strong indicator that the refactoring achieves its goals
  5. Good Test Coverage

    • All test files updated to match new signatures
    • Pre-commit hooks passed (linting, formatting, type checking, security, tests)

🔍 Code Quality Observations

Minor: Import Organization (src/celeste/streaming.py:9-10)
The renaming of Chunk to ChunkBase via import alias is functional but could be clearer:

from celeste.io import Chunk as ChunkBase
from celeste.io import Output

Consider whether this alias is necessary or if the base class could have a distinct name (though this may be out of scope for this PR).

Good: Variance Annotation (src/celeste/streaming.py:14)
The generic signature Stream[Out: Output, Params: Parameters, Chunk: ChunkBase] properly constrains type parameters with upper bounds, ensuring type safety.

Observation: Client Return Types (src/celeste/client.py:78, 163)
The changes to Client.stream() and _stream_class() return types now include the Chunk parameter:

  • Line 78: -> Stream[Out, Params, Chunk]
  • Line 163: -> type[Stream[Out, Params, Chunk]]

However, the Client class itself doesn't have a Chunk type parameter. The code uses the base Chunk from imports (line 18). This works because:

  1. Each capability-specific client subclass will return its own specific Stream type
  2. The base Client class uses the generic Chunk type as a placeholder

This is acceptable but worth noting for future maintainability.

🎯 Best Practices

Single Responsibility: Each change serves the single purpose of improving type safety
Backward Compatibility: The changes are additive to the type system without breaking runtime behavior
Documentation: PR description clearly explains benefits and changes
Testing: All tests updated and passing

🔒 Security Considerations

✅ No security concerns identified
✅ Bandit security checks passed
✅ No changes to authentication, authorization, or data handling

Performance Considerations

✅ No performance impact - this is a compile-time type system change
✅ No changes to runtime logic or data structures
✅ No additional allocations or processing

🐛 Potential Issues

None identified. The implementation appears solid.

📝 Suggestions

  1. Consider Documentation Update (optional)

    • If there's API documentation for the Stream class, consider updating it to explain the Chunk type parameter
    • Examples showing how to properly subclass Stream with specific chunk types would be helpful for future contributors
  2. Test Coverage Note

    • The test changes in test_streaming.py correctly update type signatures
    • Consider adding a test that validates type checking works correctly (though this might be better suited for integration tests)

🎉 Conclusion

This is a high-quality refactoring that achieves its stated goals:

  • ✅ Type-safe chunk parsing per capability
  • ✅ Better IDE autocomplete and type checking
  • ✅ Cleaner provider code
  • ✅ Improved mypy compliance

The changes are well-structured, consistently applied, and properly tested. The removal of mypy ignores for streaming modules is particularly notable as evidence of success.

Recommendation: Approve and merge

Great work on improving the type safety of the codebase! This will make the streaming API much more pleasant to work with.

@Kamilbenkirane Kamilbenkirane merged commit fb3cbbb into main Nov 17, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant